fixes edge cases in Android Native App rendering by mgood7123 · Pull Request #32 · DiligentGraphics/DiligentTools

mgood7123 · 2021-06-27T18:25:46Z

adapted from https://github.com/151706061/OpenGLSamples/blob/1e871c0c7fe0d8d30825f467bfdc8de50961b6f9/extensions/src/NvAppBase/NvAndroidNativeAppGlue.c#L128

this fixes possible rendering edge cases mentioned in https://developer.nvidia.com/fixing-common-android-lifecycle-issues-games

this additionally fixes an uncommon crash when the assistant is brought up over the application
this additionally fixes an uncommon crash when the application is switched from the top split screen to the bottom split screen, in Samsung One UI devices

TheMostDiligent · 2021-06-27T18:36:49Z

NativeApp/include/Android/AndroidAppBase.hpp

What is this for? Nothing waits on this conditional

TheMostDiligent · 2021-06-27T18:37:14Z

NativeApp/include/Android/AndroidAppBase.hpp

It is better to use std::mutex

ok, though i think std::mutex is slightly slower than pthread_mutex in terms of overhead, but i will switch to std::mutex

TheMostDiligent · 2021-06-27T18:37:39Z

NativeApp/include/Android/AndroidAppBase.hpp

No need for NV_ prefix

TheMostDiligent · 2021-06-27T18:38:32Z

NativeApp/include/Android/AndroidAppBase.hpp

lifecycle_flags_ to be consistent with the rest

TheMostDiligent · 2021-06-27T18:38:48Z

NativeApp/include/Android/AndroidAppBase.hpp

window_width_

TheMostDiligent · 2021-06-27T18:40:08Z

NativeApp/src/Android/AndroidAppBase.cpp

No need to initialize if you use std::mutex

TheMostDiligent · 2021-06-27T18:40:22Z

NativeApp/src/Android/AndroidAppBase.cpp

Why is it an error?

to make it stand out in the log as it is important for seeing what is going on with the state of the native app internally when debugging the state, since errors have red text in logcat

TheMostDiligent · 2021-06-27T18:40:43Z

NativeApp/src/Android/AndroidAppBase.cpp

Delete this?

TheMostDiligent · 2021-06-27T18:40:53Z

NativeApp/src/Android/AndroidAppBase.cpp

to make it stand out in the log as it is important for seeing what is going on with the state of the native app internally when debugging the state, since errors have red text in logcat

TheMostDiligent · 2021-06-27T18:41:30Z

NativeApp/src/Android/AndroidAppBase.cpp

std::lock_guard lock{mutex}; here and in similar places

TheMostDiligent

There are 11 commits in this branch already. Please squash them into one.

NativeApp/include/Android/AndroidAppBase.hpp

TheMostDiligent · 2021-06-27T20:00:24Z

NativeApp/include/Android/AndroidAppBase.hpp

APP_STATUS_FLAGS app_status_ = APP_STATUS_FLAG_NONE

TheMostDiligent · 2021-06-27T20:01:31Z

NativeApp/src/Android/AndroidAppBase.cpp

"Errors.hpp", this is not a system header

TheMostDiligent · 2021-06-27T20:02:07Z

NativeApp/src/Android/AndroidAppBase.cpp

lock{eng->mutex}

TheMostDiligent · 2021-06-27T20:04:39Z

NativeApp/include/Android/AndroidAppBase.hpp

I suggest give it a name (e.g. APP_STATUS_FLAGS) and use DEFINE_FLAG_ENUM_OPERATORS(APP_STATUS_FLAGS)

TheMostDiligent · 2021-06-27T20:08:25Z

NativeApp/src/Android/AndroidAppBase.cpp

Why extra message?

TheMostDiligent · 2021-06-27T20:08:38Z

NativeApp/src/Android/AndroidAppBase.cpp

Why extra message?

TheMostDiligent · 2021-06-27T20:08:51Z

NativeApp/src/Android/AndroidAppBase.cpp

Why extra message?

TheMostDiligent · 2021-06-27T20:09:07Z

NativeApp/src/Android/AndroidAppBase.cpp

Why extra message?

TheMostDiligent · 2021-06-27T20:11:49Z

NativeApp/src/Android/AndroidAppBase.cpp

This does not look pretty.
(app_status_ & APP_STATUS_FLAG_ACTIVE) != APP_STATUS_FLAG_NONE
Would be clearer

mgood7123 · 2021-06-28T02:20:35Z

There are 11 commits in this branch already. Please squash them into one.

is it possible to do this via github.com or would i manually need to do some git command line kung fu? (eg git reset, git add, git force push)

TheMostDiligent · 2021-06-28T02:26:30Z

You can force-push to the PR branch

mgood7123 · 2021-06-28T02:29:32Z

You can force-push to the PR branch

ok

mgood7123 · 2021-06-28T03:24:07Z

apparently force pushing a branch with no changes closes the pull request...

TheMostDiligent · 2021-06-28T03:47:52Z

NativeApp/src/Android/AndroidAppBase.cpp

I actually think it makes sense to always log this - there is really no reason not to...
Same for other places.

so this?

case APP_CMD_INIT_WINDOW: LOG_INFO_MESSAGE("APP_CMD_INIT_WINDOW");

logging of all events

how is this? 60f08d8

mgood7123 · 2021-06-28T05:10:25Z

is this better?

mgood7123 · 2021-07-04T22:59:55Z

NativeApp/include/Android/AndroidAppBase.hpp

this variable is unused, i need to remove it when i have time

mgood7123 · 2021-07-04T23:00:04Z

NativeApp/include/Android/AndroidAppBase.hpp

this variable is unused, i need to remove it when i have time

Squashed commit of the following: commit ba7271f Author: mgood7123 <smallville7123@gmail.com> Date: Sat Jul 3 03:12:45 2021 +1000 use std::atomic instead of std::mutex commit 54e3973 Author: mgood7123 <smallville7123@gmail.com> Date: Tue Jun 29 07:52:35 2021 +1000 fixes edge cases in Android Native App rendering Revert "fix possible rendering edge cases" This reverts commit 3d76fb8. fix possible rendering edge cases Update AndroidMain.cpp add functions for app status apply validator suggestions Update AndroidAppBase.hpp Update AndroidAppBase.cpp remove unused variables

TheMostDiligent requested changes Jun 27, 2021

View reviewed changes

mgood7123 requested a review from TheMostDiligent June 27, 2021 19:31

TheMostDiligent requested changes Jun 27, 2021

View reviewed changes

TheMostDiligent requested a review from azhirnov June 28, 2021 02:27

mgood7123 force-pushed the patch-1 branch 2 times, most recently from be09133 to 96ead7d Compare June 28, 2021 03:10

mgood7123 closed this Jun 28, 2021

mgood7123 force-pushed the patch-1 branch from 96ead7d to 7ecb18d Compare June 28, 2021 03:13

mgood7123 reopened this Jun 28, 2021

mgood7123 force-pushed the patch-1 branch 5 times, most recently from 58a9e7e to 2d43095 Compare June 28, 2021 03:45

TheMostDiligent reviewed Jun 28, 2021

View reviewed changes

mgood7123 force-pushed the patch-1 branch 2 times, most recently from 60f08d8 to 3d76fb8 Compare June 28, 2021 04:51

mgood7123 force-pushed the patch-1 branch 7 times, most recently from 67c0830 to e09d30c Compare July 2, 2021 23:31

mgood7123 commented Jul 4, 2021

View reviewed changes

mgood7123 force-pushed the patch-1 branch from e09d30c to ac219fa Compare July 6, 2021 06:08

Conversation

mgood7123 commented Jun 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgood7123 Jun 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheMostDiligent left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgood7123 commented Jun 28, 2021

Uh oh!

TheMostDiligent commented Jun 28, 2021

Uh oh!

mgood7123 commented Jun 28, 2021

Uh oh!

mgood7123 commented Jun 28, 2021

Uh oh!

TheMostDiligent Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgood7123 Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgood7123 Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgood7123 commented Jun 27, 2021 •

edited

Loading

mgood7123 Jun 27, 2021 •

edited

Loading

TheMostDiligent Jun 28, 2021 •

edited

Loading

mgood7123 Jun 28, 2021 •

edited

Loading

mgood7123 Jun 28, 2021 •

edited

Loading

mgood7123 Jul 4, 2021 •

edited

Loading

mgood7123 Jul 4, 2021 •

edited

Loading